uno: zeroize per-IO DMA buffers on teardown using the GDMA engine - #632
uno: zeroize per-IO DMA buffers on teardown using the GDMA engine#632Rajib Dutta (radutta99) wants to merge 7 commits into
Conversation
Internal provisioning (partition identity, enable-time keygen, boot-key masking, vault teardown) runs on the reserved admin IO slot rather than a host IO, so it never reaches drop_io. Its scratch was only watermark- rewound, never wiped, leaving raw private-key material (e.g. the P-384 identity scalar) resident in the slot indefinitely. Add UnoHsmPal::with_admin_io, the admin-side counterpart of drop_io: it opens an admin IO plus a rewound scoped allocator, runs the caller's async closure, then scrubs the slot. Binding the scrub to the session rather than to each caller means no admin path can forget it, and the higher-ranked bound stops any scratch from escaping past the scrub. ensure_unwrapping_key_imported is left as-is: it is a synchronous fn whose atomicity against the cooperative scheduler is load-bearing, and it feeds the &'static GSRAM blob straight into the vault with no DMA-scratch copy. Also fix a clippy manual_is_multiple_of warning in cpu_zeroize.
`with_admin_io`'s higher-ranked bound stops the closure returning anything borrowed from the session's scoped allocator, and the doc claimed that meant no scratch could outlive the scrub. That was not quite true: the closure captures the PAL, and the PAL-level `dma_alloc` hands back a buffer whose lifetime is tied to the PAL, not to the session. A closure could return one and the caller would silently receive memory the scrub had just zeroed. Add `R: 'static`. Every caller already returns an owned value, so this costs nothing, and it turns the doc claim into something the compiler checks: an attempt to return a PAL-level buffer out of the session now fails to compile with "returning this value requires that `'1` must outlive `'static`". Also record why `ensure_unwrapping_key_imported` still takes a raw admin IO instead of a session: it is synchronous, so it cannot await a scrub, and it allocates no scratch — `create_sync` copies from the `&'static` GSRAM slot straight into vault storage — so there is nothing to wipe. Without the note it reads like an omission. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
The teardown scrub wiped each slot's full capacity — 18 KiB of SRAM and 1.5 KiB of DTCM — however little the command touched. The live bump watermark cannot bound that, because it rewinds on every scope exit, so track a per-(slot, heap) peak that only grows and wipe to that instead. The peak is cleared by the scrub rather than by the allocator reset, so it always means "bytes written since the last scrub". Everything past it is already zero from that scrub, and an IO that ends without one keeps its peak, so the next scrub still covers it. `dma_alloc_var*` need care: they reserve the whole remaining heap so the callback need not know its encoded length up front, which would pin the peak at capacity on every IO. They now snapshot the peak before the reservation and restore it to the trimmed length, and document the contract this rests on — the callback must not write past the length it returns. On the error path the peak is left at the full reservation, since a partial write before the failure is unbounded. Measured on hardware over ~1.7k IOs: the DMA peak averages ~933 B of the 18 KiB slot with a maximum of ~3.3 KiB, so the wipe now fits in a single DUMMY_MEM-sized GDMA transfer instead of two. Instrumenting the scrub to scan each slot's whole capacity afterwards found no non-zero byte past the recorded peak, on any slot, in any run. Also in this change: - `zeroize_mem` wipes through the GDMA engine rather than the CPU, falling back to a volatile CPU wipe if the engine has no free tag. Every `DmaBuf` is GSRAM-backed by construction — the DTCM heap hands out plain `&mut [u8]` — so the destination is always GDMA-reachable. - `UnoHsmIo::admin` becomes `admin_no_scrub`, so a path that opts out of the scrubbed session has to say so. Its one caller is the synchronous unwrapping-key import, which cannot await a scrub and writes nothing to either bump heap. - `delete_key` takes the caller's admin IO instead of opening a session per key. `KeyVault::delete` takes no allocator, so it cannot dirty the heaps, and `clear_enabled_state` deletes one key per provisioning slot and per live session — a session each would have scrubbed the slot every time for no benefit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
There was a problem hiding this comment.
Pull request overview
This PR adds reliable zeroization of Uno’s per-IO scratch heaps on IO teardown, primarily using the GDMA engine by copying from a read-as-zero “Dummy Memory” window, and extends the same scrubbing guarantees to internal/admin provisioning flows that don’t naturally pass through host IO teardown.
Changes:
- Model the SoC “Dummy Memory” read-as-zero region in RDL/reggen and use it as the GDMA zero source for SRAM wipes.
- Add per-IO high-water tracking and scrub both SRAM (via GDMA, CPU fallback) and DTCM (CPU volatile wipe) on
drop_ioand admin-session exit. - Refactor admin provisioning paths to run under
with_admin_io(...)so the admin IO slot is scrubbed consistently (with a deliberateadmin_no_scrubopt-out).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| fw/plat/uno/rdl/soc/uno.rdl | Adds Dummy Memory region to the Uno SoC map and includes its RDL. |
| fw/plat/uno/rdl/soc/dummy_mem.rdl | New RDL defining the read-as-zero Dummy Memory window used as GDMA wipe source. |
| fw/plat/uno/fw/reg/soc/src/lib.rs | Exposes the generated dummy_mem register module. |
| fw/plat/uno/fw/reg/soc/src/dummy_mem.rs | New autogenerated constants/regs for Dummy Memory base/size. |
| fw/plat/uno/fw/pal/src/part.rs | Routes admin/provisioning flows through with_admin_io to ensure admin-slot scrubbing; introduces admin_no_scrub usage for the synchronous path. |
| fw/plat/uno/fw/pal/src/pal.rs | Adds io_peak tracking alongside io_alloc to bound teardown scrubs. |
| fw/plat/uno/fw/pal/src/io.rs | Scrubs per-IO scratch on drop_io; introduces with_admin_io and renames admin IO constructor to admin_no_scrub. |
| fw/plat/uno/fw/pal/src/gdma.rs | Implements GDMA-based zeroization via Dummy Memory copy, adds scrub logic + CPU volatile fallback. |
| fw/plat/uno/fw/pal/src/alloc.rs | Adds peak-watermark tracking and helpers to compute per-slot dirty prefixes; adjusts dma_alloc_var* to preserve accurate peaks. |
Suppressed comments (1)
fw/plat/uno/fw/pal/src/alloc.rs:559
- Same issue as
dma_alloc_var:lenis clamped for watermark/peak accounting, but&mut buf[..len]is not clamped and can panic iffreturns a larger value than the reserved buffer length. Clamp the returned slice length as well.
let end = start + len.min(buf.len());
w.with(|v| *v = end);
pk(self, io_index, DMA).with(|v| *v = peak_before.max(end));
// SAFETY: `buf` came from the SRAM Dma pool.
Ok((unsafe { DmaBuf::from_raw_mut(&mut buf[..len]) }, extra))
`dma_alloc_var*` clamped the callback's `len` when advancing the watermark but then sliced `buf[..len]` unclamped, so the two disagreed and a callback that reported more than it was handed would panic — taking the firmware down rather than failing the one command. Reject it with `InvalidArg` instead, as the std PAL already does: refuse to hand back a longer slice than we own. The watermark rewinds, but the peak stays at the full reservation, since a callback that over-reports wrote an unknown amount and the teardown scrub has to cover it. Reported by the Copilot reviewer on #632. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fw/plat/uno/fw/pal/src/gdma.rs:250
gdma_zero_regionchunks the transfer up toDUMMY_MEM_SIZE(16 KiB), but the GDMA driver documents inline SGL Data Block descriptors as max 4 KiB per transfer (fw/plat/uno/fw/drivers/gdma/src/types.rs). If an IO’s dirty region ever exceeds 4 KiB (e.g., the error paths that intentionally leaveio_peakat the full reservation), the first 16 KiB copy is likely to fail and force the CPU fallback + warning on every such teardown. Limit each GDMA copy to 4 KiB (and optionally still cap byDUMMY_MEM_SIZE).
/// Copies zeros from the [`DUMMY_MEM_BASE`] window, chunked to its
/// [`DUMMY_MEM_SIZE`] so an arbitrarily large region can be wiped
/// with a fixed 16 KiB zero source. Both operands use the device
/// interface. Only valid for GDMA-reachable memory (GSRAM); the M7 TCM
/// is not on the GDMA fabric and must be wiped by the CPU.
async fn gdma_zero_region(&self, dst_ptr: *mut u8, len: usize) -> HsmResult<()> {
let mut off = 0usize;
while off < len {
let chunk = core::cmp::min(len - off, DUMMY_MEM_SIZE as usize);
let src = device_dma_buf(DUMMY_MEM_BASE as *const u8, chunk as u32);
`InvalidArg` was the wrong status for a callback that reports writing more than the buffer it was handed. The enum's own docs reserve `InvalidArg` for a caller bug — "unknown id, kind mismatch", "a malformed request" — but here the request is fine and the fault is ours: an encoder mis-reporting its output length. Returning `InvalidArg` blames the host for a firmware bug. Add `DmaAllocLenOverrun`, following the `UndoLogFull` precedent of a dedicated, documented status for a should-never-happen firmware bug, and use it from both PALs so they report the same thing for the same condition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
| //! | `IO_META[index]` | 8B metadata | Controller/queue IDs from IIC recv | | ||
| //! | `DTCM_IO_BUF[index]` | 1.5KB fmem | Fast DTCM workspace buffer | | ||
| //! | `SRAM_IO_BUF[index]` | 8KB smem | Large SRAM workspace buffer | | ||
| //! | `SRAM_IO_BUF[index]` | 18KB smem | Large SRAM workspace buffer | |
There was a problem hiding this comment.
Did you verify that the 18KB is in error and should be 8KB instead?
There was a problem hiding this comment.
Will wait for Vishal Soni (@vsonims) on this question. On my local testing with enabled DDI tests, 8KB uno works just fine.
Also note that as per the changes in this PR, zeroization performance does NOT depend on this size. It depends on how much size the IO end up making dirty in its lifetime. So we can leave the size as is today with no efficiency loss.
Addresses review feedback: use a vetted zeroization primitive and widen the CPU store from 32-bit to 64-bit. `cpu_zeroize` was a hand-rolled 32-bit volatile-store loop. Replace it with the audited `zeroize` crate applied to a `[u64]` view of the region: an 8-byte-aligned QWORD body framed by a byte-wise head/tail for any unaligned edges. Both per-IO regions (`DTCM_IO_BUF` 0x600, `SRAM_IO_BUF` 0x4800) are 8-byte aligned and 8-multiples, so in practice it is a pure `u64` wipe with no head or tail. On Cortex-M7 `[u64]::zeroize()` lowers to `STRD` (64-bit doubleword stores), verified in the release disassembly — the wipe loop is a 4x unrolled `strd rZero, rZero, [ptr, #off]`. That halves the store count versus the previous word loop and quarters a byte-wise wipe, on the CPU fallback and the small DTCM path (the 18 KiB SRAM slot is still wiped by GDMA). `zeroize` emits the volatile writes plus an atomic fence, so the wipe still cannot be elided. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
7dbbe27 to
443ccf9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
fw/plat/uno/fw/pal/src/gdma.rs:307
scrub_io_slotlogs a warning on any GDMA failure, including expected transient resource exhaustion (e.g.GdmaError::NO_FREE_TAGS/SQ_FULL). In a busy system this can spam logs and make it harder to notice real scrub regressions; it also loses the actual error value by using.is_err().
Capture the GDMA error and only warn! on unexpected failures (still falling back to the CPU wipe for all errors).
let (dma_ptr, dma_len) = crate::alloc::io_slot_dma_dirty(self, io_index);
if dma_len != 0 && self.gdma_zero_region(dma_ptr, dma_len).await.is_err() {
// Surface the fallback: a persistent GDMA fault would otherwise
// silently degrade every wipe into a CPU loop with no signal.
// Logged via the trace facade (as the iic/oic drivers do for
What
Uno never wiped its per-IO scratch.
stdzeroizes onbuf_pool::free, but uno only rewound the bump watermark, so key material — RSA private keys, the P-384 identity scalar, OAEP plaintext — stayed resident in the per-IO SRAM/DTCM heaps and was visible to whichever IO reused the slot. Uno has no heap, so those bump buffers are the only place such material exists.This adds a teardown scrub driven by the GDMA engine, per Jayant Gandhi (@jaygmsft)'s review comment on #602.
How
The wipe is a DMA copy from a read-as-zero window. GDMA has no fill/memset opcode — an SQ entry's
OP_CODEonly encodes read-src / write-dst — so zeroing means copying out of the SoC's "Dummy Memory" region at0xA0B0_0000. That region wasn't modelled anywhere in the firmware, so it is now an RDL entry (dummy_mem.rdl) with the base and size generated into the reg crate rather than hardcoded.Every host IO. The scrub hangs off
drop_io, which the core dispatch loop calls on both the completed and the dropped paths.Internal provisioning too. The admin slot never passes through
drop_io, so identity/enable-time keygen and boot-key masking now run inside awith_admin_iosession that scrubs on exit — obtaining an admin IO is entering a scrubbed scope.UnoHsmIo::admin_no_scrubis the one deliberate opt-out, named so a new call site has to choose it.Only what was written. A per-(slot, heap) high-water mark bounds the wipe.
dma_alloc_var*reserve the whole remaining heap so the callback need not know its encoded length up front; they snapshot the peak before reserving and restore it to the trimmed length afterwards, and document the contract that rests on. On the error path the peak stays at the full reservation, since a partial write before a failure is unbounded.Hardware validation
Flashed to the Manticore EVB and instrumented to scan each slot's whole capacity after every scrub, classifying any non-zero byte as inside or beyond the wiped region:
dma_alloc_varcontract holds for both the MBOR encoder and the TBOR frame builders.Cost: the DMA peak averages ~933 B of the 18 KiB slot, max ~3.3 KiB — so the wipe fits in a single
DUMMY_MEM-sized GDMA transfer instead of two.Regression: 45/45 over three passes (cert-chain incl. multithread, ECC-sign compat across P-256/384/521, sign/verify, part-info).
rsa_unwrapandattest_keymatch this branch's baseline exactly; their failures are pre-existing bring-up gaps (UnsupportedCmdfor the PKA-stubbed ECC/RSA attestation,FileHandleNoExistingSessionfor NoSession routing).Notes for review
io_peakstarts at zero, so each slot's first scrub covers only what that boot wrote. That relies on the IO heaps arriving clean, which 1SP guarantees on a warm boot; the field doc records the dependency and what to change if that ever stops holding.Merge order
#602 removes its local zeroize calls on the assumption this lands, so if #602 merges first there is a window with no wipe on those paths. Worth sequencing this one first.